-
-
Notifications
You must be signed in to change notification settings - Fork 949
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
fix: Specify size for the SpriteWidget #1760
fix: Specify size for the SpriteWidget #1760
Conversation
child: CustomPaint(painter: SpritePainter(sprite, anchor, angle: angle)), | ||
child: CustomPaint( | ||
painter: SpritePainter(sprite, anchor, angle: angle), | ||
size: sprite.srcSize.toSize(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will prevent the user to render the sprite bigger than its original size right?
I think that a better approach here would be to use the BoxFit enum. Or, maybe something like that. wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be considered a feature request: adding explicit size
parameter to SpriteWidget, and then taking it into account within SpriteRenderer.
Current PR only addresses the problem that SpriteWidget is currently rendered at size 0, which is problematic if someone tries to do layout.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I get that, but this will solve one issue, while adding a new one, where all sprite widgets will always have the size of the image. And it will be impossible to render it bigger than its original size.
So we need an alternative here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, i'm looking at RenderCustomPaint
render-box, and it says that it considers the provided size as the preferred size, meaning it will attempt to find the size that is as close as possible to the given size, while still satisfying the parent's constraints. Meaning that if constrains say that minSize=maxSize=100, then that will be size of the widget regardless of what its size
property says.
I've tried this with the SpriteWidgetExample, and it can still scale the sprite both up and down.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, i'm looking at
RenderCustomPaint
render-box, and it says that it considers the provided size as the preferred size, meaning it will attempt to find the size that is as close as possible to the given size, while still satisfying the parent's constraints. Meaning that if constrains say that minSize=maxSize=100, then that will be size of the widget regardless of what itssize
property says.I've tried this with the SpriteWidgetExample, and it can still scale the sprite both up and down.
Interesting! So my concern will not happen! Thanks for checking on that. LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting indeed, can someone open an issue for setting custom sizes?
…flame-engine#1761) This PR changes the signature of the closeToVector() helper function: Old: closeToVector(num x, num y, {double epsilon = 1e-15}) New: closeToVector(Vector2 vector, [double epsilon = 1e-15])
/// ``` | ||
Matcher closeToVector(num x, num y, {double epsilon = 1e-15}) { | ||
return _IsCloseTo(Vector2(x.toDouble(), y.toDouble()), epsilon); | ||
Matcher closeToVector(Vector2 vector, [double epsilon = 1e-15]) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not be part of this PR! It is a breaking change of non-related feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that GitHub is confused, this was already merged in #1761
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now the PR looks like it should again.
Description
Set
size
property for theSpriteWidget
Checklist
fix:
,feat:
,docs:
etc).docs
and added dartdoc comments with///
.examples
.Breaking Change
Related Issues
Closes #1575